Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use &raw in A|Rc::as_ptr #73845

Merged
merged 10 commits into from
Jul 4, 2020
Merged

Use &raw in A|Rc::as_ptr #73845

merged 10 commits into from
Jul 4, 2020

Conversation

CAD97
Copy link
Contributor

@CAD97 CAD97 commented Jun 28, 2020

This PR uses &raw for offsetting *mut [A]RcInner<T> -> *mut T.

Additionally, this updates the implementation of Weak::as_ptr to support unsized T, though it does not yet relax the bounds of Weak::as_ptr/into_raw/from_raw to accept unsized T.

@rust-highfive
Copy link
Collaborator

r? @sfackler

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 28, 2020
@sfackler
Copy link
Member

r? @RalfJung

@rust-highfive rust-highfive assigned RalfJung and unassigned sfackler Jun 28, 2020
src/liballoc/rc.rs Outdated Show resolved Hide resolved
src/liballoc/rc.rs Outdated Show resolved Hide resolved
@CAD97
Copy link
Contributor Author

CAD97 commented Jun 30, 2020

Addressed the review comments (and in sync.rs as well, of course).

(I really need to figure out why format-on-save is not agreeing with ./x.py fmt....)

@CAD97 CAD97 force-pushed the weak-as-unsized-ptr branch from dce66a3 to b581b39 Compare June 30, 2020 16:51
@CAD97 CAD97 force-pushed the weak-as-unsized-ptr branch from b581b39 to aed88e1 Compare June 30, 2020 19:03
src/liballoc/rc.rs Outdated Show resolved Hide resolved
src/liballoc/rc.rs Outdated Show resolved Hide resolved
CAD97 and others added 2 commits July 1, 2020 15:05
@CAD97 CAD97 force-pushed the weak-as-unsized-ptr branch from 31f8489 to 98789ac Compare July 1, 2020 19:06
src/liballoc/rc.rs Outdated Show resolved Hide resolved
@CAD97 CAD97 force-pushed the weak-as-unsized-ptr branch from 0db0dcb to 1b5ac57 Compare July 2, 2020 21:08
@RalfJung
Copy link
Member

RalfJung commented Jul 3, 2020

Awesome, thanks a lot for enduring all these rounds of review. :)
@bors r+

@bors
Copy link
Contributor

bors commented Jul 3, 2020

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 3, 2020
@RalfJung
Copy link
Member

RalfJung commented Jul 3, 2020

Second network failure in a row? Cc @rust-lang/infra
@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 3, 2020
@RalfJung
Copy link
Member

RalfJung commented Jul 3, 2020

Same failure in 2 other PRs.
@bors treeclosed=100

@pietroalbini
Copy link
Member

Opened #73992 to track it, will ping GitHub.

bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 4, 2020
…arth

Rollup of 12 pull requests

Successful merges:

 - rust-lang#73140 (Fallback to xml.etree.ElementTree)
 - rust-lang#73670 (Add `format_args_capture` feature)
 - rust-lang#73693 (Use exhaustive match in const_prop.rs)
 - rust-lang#73845 (Use &raw in A|Rc::as_ptr)
 - rust-lang#73861 (Create E0768)
 - rust-lang#73881 (Standardize bibliographic citations in rustc API docs)
 - rust-lang#73925 (Improve comments from rust-lang#72617, as suggested by RalfJung)
 - rust-lang#73949 ([mir-opt] Fix mis-optimization and other issues with the SimplifyArmIdentity pass)
 - rust-lang#73984 (Edit docs for rustc_data_structures::graph::scc)
 - rust-lang#73985 (Fix "getting started" link)
 - rust-lang#73997 (fix typo)
 - rust-lang#73999 (Bump mingw-check CI image from Ubuntu 16.04 to 18.04.)

Failed merges:

 - rust-lang#74000 (add `lazy_normalization_consts` feature gate)

r? @ghost
@bors bors merged commit 9a659c5 into rust-lang:master Jul 4, 2020
@CAD97 CAD97 deleted the weak-as-unsized-ptr branch July 4, 2020 05:02
CAD97 added a commit to CAD97/rust that referenced this pull request Jul 4, 2020
....to allow use with unsized T. This is a follow-up to rust-lang#73845,
which did the impl work required to be able to relax these bounds.
@RalfJung RalfJung mentioned this pull request Jul 27, 2020
3 tasks
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Oct 3, 2020
Allow Weak::as_ptr and friends for unsized T

Relaxes `impl<T> Weak<T>` to `impl<T: ?Sized> Weak<T>` for the methods `rc::Weak::as_ptr`, `into_raw`, and `from_raw`.

Follow-up to rust-lang#73845, which did most of the impl work to make these functions work for `T: ?Sized`.

We still have to adjust the implementation of `Weak::from_raw` here, however, because I missed a use of `ptr.is_null()` previously. This check was necessary when `into`/`from_raw` were first implemented, as `into_raw` returned `ptr::null()` for dangling weak. However, we now just (wrapping) offset dangling weaks' pointers the same as nondangling weak, so the null check is no longer necessary (or even hit). (I can submit just 17a928f as a separate PR if desired.)

As a nice side effect, moves the `fn is_dangling` definition closer to `Weak::new`, which creates the dangling weak.

This technically stabilizes that "something like `align_of_val_raw`" is possible to do. However, I believe the part of the functionality required by these methods here -- specifically, getting the alignment of a pointee from a pointer where it may be dangling iff the pointee is `Sized` -- is uncontroversial enough to stabilize these methods without a way to implement them on stable Rust.

r? @RalfJung, who reviewed rust-lang#73845.

ATTN: This changes (relaxes) the (input) generic bounds on stable fn!
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 3, 2020
Allow Weak::as_ptr and friends for unsized T

Relaxes `impl<T> Weak<T>` to `impl<T: ?Sized> Weak<T>` for the methods `rc::Weak::as_ptr`, `into_raw`, and `from_raw`.

Follow-up to rust-lang#73845, which did most of the impl work to make these functions work for `T: ?Sized`.

We still have to adjust the implementation of `Weak::from_raw` here, however, because I missed a use of `ptr.is_null()` previously. This check was necessary when `into`/`from_raw` were first implemented, as `into_raw` returned `ptr::null()` for dangling weak. However, we now just (wrapping) offset dangling weaks' pointers the same as nondangling weak, so the null check is no longer necessary (or even hit). (I can submit just 17a928f as a separate PR if desired.)

As a nice side effect, moves the `fn is_dangling` definition closer to `Weak::new`, which creates the dangling weak.

This technically stabilizes that "something like `align_of_val_raw`" is possible to do. However, I believe the part of the functionality required by these methods here -- specifically, getting the alignment of a pointee from a pointer where it may be dangling iff the pointee is `Sized` -- is uncontroversial enough to stabilize these methods without a way to implement them on stable Rust.

r? `@RalfJung,` who reviewed rust-lang#73845.

ATTN: This changes (relaxes) the (input) generic bounds on stable fn!
@@ -2279,13 +2292,6 @@ unsafe fn data_offset<T: ?Sized>(ptr: *const T) -> isize {
unsafe { data_offset_align(align_of_val(&*ptr)) }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops this line should have been align_of_val_raw now, right?

But I doubt that that's enough to fix #80365.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it definitely should have been. I don't know how I missed this one 🙃

CAD97 added a commit to CAD97/rust that referenced this pull request Dec 26, 2020
This was missed in rust-lang#73845 when switching to use the raw operators.
Fixes rust-lang#80365
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Dec 28, 2020
Use raw version of align_of in rc data_offset

This was missed in rust-lang#73845 when switching to use the raw operators.
Fixes rust-lang#80365
@cuviper cuviper added this to the 1.46 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants